Skip to content

MDEV-35189: Updating cache for INNODB_LOCKS et al is suboptimal - #3584

Merged
dr-m merged 1 commit into
10.6from
10.6-MDEV-35189
Nov 12, 2024
Merged

MDEV-35189: Updating cache for INNODB_LOCKS et al is suboptimal#3584
dr-m merged 1 commit into
10.6from
10.6-MDEV-35189

Conversation

@dr-m

@dr-m dr-m commented Oct 17, 2024

Copy link
Copy Markdown
Contributor
  • The Jira issue number for this PR is: MDEV-35189

Description

There are some performance issues with the cache that is underlying the INFORMATION_SCHEMA tables INNODB_LOCKS, INNODB_LOCK_WAITS and INNODB_TRX.

ha_storage_put_memlim(): Invoke my_crc32c() to "fold", and traverse the hash table only once.

fold_lock(): Remove some redundant conditions and use my_crc32c() instead of ut_fold_ulint_pair().

trx_i_s_cache_t::add(): Replaces add_lock_to_cache(), search_innodb_locks(), and locks_row_eq_lock(). Avoid duplicated traversal of the hash table.

Release Notes

The performance of some InnoDB hash table operations was slightly improved.

How can this PR be tested?

./mtr --parallel=auto innodb.innodb_i_s_innodb_trx innodb.innodb_i_s_innodb_locks

Basing the PR against the correct MariaDB version

  • This is a new feature or a refactoring, and the PR is based against the main branch.
  • This is a bug fix, and the PR is based against the earliest maintained branch in which the bug can be reproduced.

Some of this would be applicable to 10.5 as well, but we target 10.6 because of the MDEV-35190 dependency.

PR quality check

  • I checked the CODING_STANDARDS.md file and my PR conforms to this where appropriate.
  • For any trivial modifications to the PR, I am ok with the reviewer making the changes themselves.

@dr-m dr-m self-assigned this Oct 17, 2024
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@mariadb-DebarunBanerjee mariadb-DebarunBanerjee left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The improvement looks good. Added one minor suggestion.

On a independent note, it does seem to be a very good idea to get rid of the has table abstraction and pull in the search/insert code in different part of the code. This comment is not specific to this patch ... just a note.

Comment on lines +793 to +794
ut_ad((heap_no == 0xFFFF) == lock.is_table());
if (heap_no == 0xFFFF)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer the condition to be if (lock.is_table()) which should be used uniformly in all places. heap_no is not relevant for table lock and to have the value as "0xFFFF" is good but we better not create functional dependency on it.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The 0xFFFF is special only within this subsystem, returned by the function wait_lock_get_heap_no(). We have debug assertions in place (including the one that you quoted) to document this special rule. In non-debug builds, referring to heap_no == 0xFFFF should yield better performance than dereferencing lock, because that parameter is likely to reside in a register.

ha_storage_put_memlim(): Invoke my_crc32c() to "fold", and traverse
the hash table only once.

fold_lock(): Remove some redundant conditions and use my_crc32c()
instead of ut_fold_ulint_pair().

trx_i_s_cache_t::add(): Replaces add_lock_to_cache(),
search_innodb_locks(), and locks_row_eq_lock(). Avoid duplicated
traversal of the hash table.

Reviewed by: Debarun Banerjee
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants